IGNITE-24244: Support timeout in RW transactions #5209
Conversation
PakhomovAlexander
force-pushed
the
IGNITE-24244
branch
from
13f4189 to
fef58f1
Compare
PakhomovAlexander
changed the title
# Conflicts: # modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java # modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java # modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java # modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java # modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java # modules/transactions/src/integrationTest/java/org/apache/ignite/tx/distributed/ItTransactionRecoveryTest.java # modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java # modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionInflights.java # modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxCleanupRequestSender.java # modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java # modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImplTest.java
| * | ||
| * @return The future. | ||
| */ | ||
| default CompletableFuture<Void> rollbackTimeoutExceededAsync() { |
There was a problem hiding this comment.
Please explain why do we need this method in addition to common rollbackAsync().
There was a problem hiding this comment.
Ive removed the default implementation. I can't see another way to provide an information that the transactions was rolled back because of timeout exceeded.
| return rollbackAsync(); | ||
| } | ||
|
|
||
| default boolean isTimeoutExceeded() { |
There was a problem hiding this comment.
Please specify guaranties of the method in the contract. It's not clear whether the method will return true if logical timeout was exceeded or if it was detected, etc? In other words if transactions with timeout 20 was started at HLC timestamp t(10,10) will isTimeoutExceeded return true if HLC(t30, X)?
There was a problem hiding this comment.
Added java doc and changed the naming. Does it make sense?
| @@ -149,7 +149,9 @@ default InternalTransaction beginExplicitRo(HybridTimestampTracker timestampTrac | |||
| * @param ts The timestamp which is associated to txn completion. | |||
| * @param commit {@code True} if a commit requested. | |||
| */ | |||
| void finishFull(HybridTimestampTracker timestampTracker, UUID txId, @Nullable HybridTimestamp ts, boolean commit); | |||
| void finishFull( | |||
| HybridTimestampTracker timestampTracker, UUID txId, @Nullable HybridTimestamp ts, boolean commit, boolean timeoutExceeded | |||
There was a problem hiding this comment.
How does timeoutExceeded=true matches with commit = true?
There was a problem hiding this comment.
Added assertions and adjusted javadoc.
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
The Review Checklist
- There is a single JIRA ticket related to the pull request.
- The web-link to the pull request is attached to the JIRA ticket.
- The JIRA ticket has the Patch Available state.
- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
Notes